cnf-tests: Add numa tests with multiple interfaces #1590

gkopels · 2023-08-02T09:33:55Z

Test cases uses a set SriovNetworkNodePolicies and performance profile to control NUMA node placement in test.
This test creates a pod with sriov interface on NUMA node 1 with the sriov interface in NUMA node 0. The excludeTopology is set to True. THe pod is expected to be deployed.

openshift-ci · 2023-08-02T09:34:08Z

Hi @gkopels. Thanks for your PR.

I'm waiting for a openshift-kni member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zeeke · 2023-08-02T09:46:04Z

/ok-to-test

Test cases uses a set SriovNetworkNodePolicies and performance profile to control NUMA node placement in test. This test creates a pod with sriov interface on NUMA node 1 with the sriov interface in NUMA node 0. The excludeTopology is set to True. THe pod is expected to be deployed.

zeeke

Left few minor comments. Overall PR looks good to me

zeeke · 2023-08-03T11:56:38Z

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

+			"testNuma1NIC1ExcludeTopoplogyFalse", ipam, false)
+
+		createSriovNetworkAndPolicyForNumaAffinityTest(8, numa1DeviceList[0], "#4-7",
+			"test-numa-1-nic2-exclude-topology-true-", testingNode.Name,


nit:

Suggested change

"test-numa-1-nic2-exclude-topology-true-", testingNode.Name,

"test-numa-1-nic1-exclude-topology-true-", testingNode.Name,

zeeke · 2023-08-03T11:58:48Z

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

@@ -62,39 +67,52 @@ var _ = Describe("[sriov] NUMA node alignment", Ordered, func() {
 		sriovDevices, err := sriovCapableNodes.FindSriovDevices(testingNode.Name)
 		Expect(err).ToNot(HaveOccurred())

-		numa0Device, err := findDeviceOnNUMANode(testingNode, sriovDevices, "0")
+		numa0DeviceList, err = findDevicesOnNUMANode(testingNode, sriovDevices, "0")
 		Expect(err).ToNot(HaveOccurred())


Better to assert on device list length

Suggested change

Expect(err).ToNot(HaveOccurred())

Expect(len(numa0DeviceList)).To(BeNumerically(">=", 1))

Expect(err).ToNot(HaveOccurred())

Same for NUMA1

zeeke · 2023-08-03T12:00:06Z

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

 		Expect(err).ToNot(HaveOccurred())
-		By("Using NUMA0 device " + numa0Device.Name)
+		By("Using NUMA0 device1 " + numa0DeviceList[0].Name)
+		By("Using NUMA0 device2 " + numa0DeviceList[1].Name)


Please, move it in the if len(numa0DeviceList) > 1 { body

zeeke · 2023-08-03T12:07:17Z

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(actualPod.Status.QOSClass).To(Equal(corev1.PodQOSGuaranteed))
+			g.Expect(actualPod.Status.Phase).To(Equal(corev1.PodFailed))
+			g.Expect(actualPod.Status.Message).To(ContainSubstring("Resources cannot be allocated with Topology locality"))


Please, assert on Status.Reason

Suggested change

g.Expect(actualPod.Status.Message).To(ContainSubstring("Resources cannot be allocated with Topology locality"))

g.Expect(actualPod.Status.Reason).To(Equal("TopologyAffinityError"))

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

zeeke · 2023-08-07T15:03:04Z

/approve
LGTM

@SchSeba there are some pushes after your comment. Please, have a look

zeeke · 2023-08-07T16:34:45Z

/override ci/prow/e2e-gcp-ovn

ci/prow/e2e-gcp-ovn failures are not related to this PR

  [FAIL] [rfe_id:27368][performance] Network latency parameters adjusted by the Node Tuning Operator [It] [test_id:28467][crit:high][vendor:[email protected]][level:acceptance] Should contain configuration injected through the openshift-node-performance profile
  /go/src/github.com/openshift-kni/cnf-features-deploy/vendor/github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/1_performance/performance.go:1383
  [FAIL] [tuningcni] tuningcni over macvlan [It] pods with sysctl's over macvlan should be able to ping each other
  /go/src/github.com/openshift-kni/cnf-features-deploy/cnf-tests/testsuites/e2esuite/security/tuning.go:93
  [FAIL] [tuningcni] tuningcni over bond [It] pods with sysctls over bond should be able to ping each other
  /go/src/github.com/openshift-kni/cnf-features-deploy/cnf-tests/testsuites/e2esuite/security/tuning.go:137

openshift-ci · 2023-08-07T16:34:51Z

@zeeke: Overrode contexts on behalf of zeeke: ci/prow/e2e-gcp-ovn

In response to this:

/override ci/prow/e2e-gcp-ovn

ci/prow/e2e-gcp-ovn failures are not related to this PR

 [FAIL] [rfe_id:27368][performance] Network latency parameters adjusted by the Node Tuning Operator [It] [test_id:28467][crit:high][vendor:[email protected]][level:acceptance] Should contain configuration injected through the openshift-node-performance profile
 /go/src/github.com/openshift-kni/cnf-features-deploy/vendor/github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/1_performance/performance.go:1383
 [FAIL] [tuningcni] tuningcni over macvlan [It] pods with sysctl's over macvlan should be able to ping each other
 /go/src/github.com/openshift-kni/cnf-features-deploy/cnf-tests/testsuites/e2esuite/security/tuning.go:93
 [FAIL] [tuningcni] tuningcni over bond [It] pods with sysctls over bond should be able to ping each other
 /go/src/github.com/openshift-kni/cnf-features-deploy/cnf-tests/testsuites/e2esuite/security/tuning.go:137

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go

SchSeba

/lgtm
/approve

openshift-ci · 2023-08-09T11:28:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gkopels, SchSeba, zeeke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [SchSeba,zeeke]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

zeeke · 2023-08-10T07:22:43Z

/override ci/prow/e2e-gcp-ovn

openshift-ci · 2023-08-10T07:22:54Z

@zeeke: Overrode contexts on behalf of zeeke: ci/prow/e2e-gcp-ovn

In response to this:

/override ci/prow/e2e-gcp-ovn

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 2, 2023

openshift-ci bot requested review from SchSeba and serngawy August 2, 2023 09:34

gkopels force-pushed the add-numa-true-same-node branch 2 times, most recently from aa80665 to 2fc780a Compare August 2, 2023 09:35

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 2, 2023

gkopels force-pushed the add-numa-true-same-node branch from 2fc780a to 39ad6b8 Compare August 2, 2023 10:07

gkopels changed the title ~~cnf-tests: Add numa true on different node~~ WIP: cnf-tests: Add numa true on different node Aug 3, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2023

gkopels force-pushed the add-numa-true-same-node branch 3 times, most recently from 82a15e7 to 060a3dc Compare August 3, 2023 10:32

gkopels changed the title ~~WIP: cnf-tests: Add numa true on different node~~ cnf-tests: Add numa tests with multiple interfaces Aug 3, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2023

zeeke reviewed Aug 3, 2023

View reviewed changes

gkopels force-pushed the add-numa-true-same-node branch from 060a3dc to 3b3d999 Compare August 6, 2023 10:57

SchSeba reviewed Aug 6, 2023

View reviewed changes

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go Show resolved Hide resolved

gkopels force-pushed the add-numa-true-same-node branch 2 times, most recently from 64b3907 to 66fd322 Compare August 7, 2023 10:01

cnf-tests: add numa node test cases with multiple interfaces

2e4e43a

gkopels force-pushed the add-numa-true-same-node branch from 66fd322 to 2e4e43a Compare August 7, 2023 14:30

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2023

SchSeba reviewed Aug 8, 2023

View reviewed changes

cnf-tests/testsuites/e2esuite/dpdk/numa_node_sriov.go Show resolved Hide resolved

SchSeba approved these changes Aug 9, 2023

View reviewed changes

openshift-ci bot assigned SchSeba Aug 9, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2023

openshift-merge-robot merged commit 386f839 into openshift-kni:master Aug 10, 2023

zeeke mentioned this pull request Aug 31, 2023

[release-4.13] Backport of [sriov] NUMA ExcludeTopology test cases #1629

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cnf-tests: Add numa tests with multiple interfaces #1590

cnf-tests: Add numa tests with multiple interfaces #1590

gkopels commented Aug 2, 2023 •

edited

Loading

openshift-ci bot commented Aug 2, 2023

zeeke commented Aug 2, 2023

zeeke left a comment

zeeke Aug 3, 2023

gkopels Aug 3, 2023

zeeke Aug 3, 2023

gkopels Aug 6, 2023

zeeke Aug 3, 2023

gkopels Aug 3, 2023

zeeke Aug 3, 2023

gkopels Aug 6, 2023

zeeke commented Aug 7, 2023

zeeke commented Aug 7, 2023

openshift-ci bot commented Aug 7, 2023

SchSeba left a comment

openshift-ci bot commented Aug 9, 2023

zeeke commented Aug 10, 2023

openshift-ci bot commented Aug 10, 2023

	"test-numa-1-nic2-exclude-topology-true-", testingNode.Name,
	"test-numa-1-nic1-exclude-topology-true-", testingNode.Name,

	Expect(err).ToNot(HaveOccurred())
	Expect(len(numa0DeviceList)).To(BeNumerically(">=", 1))
	Expect(err).ToNot(HaveOccurred())

	g.Expect(actualPod.Status.Message).To(ContainSubstring("Resources cannot be allocated with Topology locality"))
	g.Expect(actualPod.Status.Reason).To(Equal("TopologyAffinityError"))

cnf-tests: Add numa tests with multiple interfaces #1590

cnf-tests: Add numa tests with multiple interfaces #1590

Conversation

gkopels commented Aug 2, 2023 • edited Loading

openshift-ci bot commented Aug 2, 2023

zeeke commented Aug 2, 2023

zeeke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeeke commented Aug 7, 2023

zeeke commented Aug 7, 2023

openshift-ci bot commented Aug 7, 2023

SchSeba left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Aug 9, 2023

zeeke commented Aug 10, 2023

openshift-ci bot commented Aug 10, 2023

gkopels commented Aug 2, 2023 •

edited

Loading